Enforce pre-API input validation in experiment state loader (SEC-002)#31002
Merged
Enforce pre-API input validation in experiment state loader (SEC-002)#31002
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a00e8a1-662f-48b8-aec9-dc885cececce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This reverts commit ed7de6e. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a00e8a1-662f-48b8-aec9-dc885cececce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a00e8a1-662f-48b8-aec9-dc885cececce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a00e8a1-662f-48b8-aec9-dc885cececce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8a00e8a1-662f-48b8-aec9-dc885cececce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix API call order in load_experiment_state_from_repo.cjs
Enforce pre-API input validation in experiment state loader (SEC-002)
May 8, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses SEC-002 by ensuring experiment state loader inputs are validated before any GitHub API request is made, so invalid inputs fail closed and skip remote fetches.
Changes:
- Added a
validateInputs(branch, owner, repo)gate to validate branch/repo inputs prior to callingoctokit.rest.repos.getContent. - Updated
main()to use the centralized validation and return early with the existing “starting with empty experiment state” behavior (without remote fetch) on failure. - Expanded unit tests to cover
validateInputs(...)and to assertmain()does not callgetContentwhen branch input is invalid.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/load_experiment_state_from_repo.cjs | Introduces validateInputs and moves input validation ahead of the GitHub API fetch. |
| actions/setup/js/load_experiment_state_from_repo.test.cjs | Adds direct validation tests and a main() regression test ensuring invalid inputs skip the API call. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 1
Comment on lines
+61
to
+63
| if (!owner || !repo) { | ||
| return { valid: false, error: "GITHUB_REPOSITORY is not set" }; | ||
| } |
Collaborator
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a9a2cf91-79cf-4812-8a55-5e48ca3df246 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a9a2cf91-79cf-4812-8a55-5e48ca3df246 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/a9a2cf91-79cf-4812-8a55-5e48ca3df246 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented and pushed follow-up updates addressing the review feedback on strict repository validation before API use in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The safe-outputs conformance check flagged
load_experiment_state_from_repo.cjsfor callingoctokit.rest.repos.getContentbefore validating request inputs. This change moves validation ahead of the API call and hardens branch/repo input checks.Pre-fetch validation added in loader
validateInputs(branch, owner, repo, repository)inactions/setup/js/load_experiment_state_from_repo.cjs.main()now validatesGH_AW_EXPERIMENT_BRANCHand rawGITHUB_REPOSITORYbefore invokingfetchFileFromBranch(...)...sequences.owner/repoformat with allowlisted characters, rejecting malformed values (whitespace, missing parts, extra segments) before API usage.Main flow updated to fail closed before remote fetch
Focused test coverage expanded
validateInputs(...)covering valid input, empty branch, invalid branch chars, traversal-like branch, malformed repository formats, and missing owner/repo scenarios.main()assertions that invalid branch input and invalidGITHUB_REPOSITORYformat do not callgetContent.